-
-
Notifications
You must be signed in to change notification settings - Fork 147
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: issue with providing str for bytes in calldata [APE-667] #1319
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! I can't approve it won't let me, just run the linters! thank you :)
Note: in debuging the implementation for this with @Ninjagod1251, I figured out that ApeWorX/ethpm-types#63 would be very helpful for some of the code we added |
# NOTE: An array can be an array of tuples, so we start with an array check | ||
if abi_type.type.endswith("]"): | ||
# remove one layer of the potential onion of array | ||
new_type = "[".join(str(abi_type.type).split("[")[:-1]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the code that I think would be better to fix in ethpm_types.abi.ABIType
by adding a method to fetch the inner array type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do itttttt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
empty raise is not legal unless an exception is already present on the Exception stack
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved
Fix the linting issue and can merge |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's gooooo
What I did
Just opening this to show @Ninjagod1251 what to do and get it started.
See comments for more info!
fixes: #1289
fixes: APE-627
How I did it
How to verify it
Checklist